-
Notifications
You must be signed in to change notification settings - Fork 516
DEBUG: limit workflow #2390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev/aio-connector
Are you sure you want to change the base?
DEBUG: limit workflow #2390
Conversation
355bc1c
to
5ec89c3
Compare
5531b8e
to
5168b00
Compare
042c5ab
to
b2f1bb9
Compare
5168b00
to
9a188b1
Compare
b2f1bb9
to
d5ee2e4
Compare
c37c333
to
f0d9e29
Compare
098c4d2
to
b2719d0
Compare
f0d9e29
to
449d668
Compare
b2719d0
to
dcfda65
Compare
d56b698
to
b919832
Compare
2b513fa
to
edd918d
Compare
{ | ||
k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" | ||
for (k, v) in body["data"].items() | ||
}, |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
sensitive data (password)
This expression logs
sensitive data (password)
This expression logs
sensitive data (password)
This expression logs
sensitive data (password)
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 21 days ago
To fix the problem, we should ensure that sensitive fields such as "PASSCODE", "password", and any other known sensitive keys are always masked in logs, regardless of their presence in any whitelist. This can be achieved by defining a set of sensitive keys and updating the dictionary comprehension in the logger statement to mask these keys. The fix should be applied in the authenticate
method of the Auth
class, specifically at the logging statement on lines 147–153. No changes to existing functionality are required, only the logging output is made safer. We will define a set of sensitive keys within the method (or at the top of the file if appropriate) and update the logging statement accordingly.
-
Copy modified lines R147-R148 -
Copy modified line R152
@@ -146,2 +146,4 @@ | ||
|
||
# Always mask sensitive fields in logs, regardless of whitelist | ||
SENSITIVE_KEYS = {"PASSCODE", "password", "pwd", "secret", "token"} | ||
logger.debug( | ||
@@ -149,3 +151,3 @@ | ||
{ | ||
k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******" | ||
k: "******" if k.upper() in SENSITIVE_KEYS else (v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******") | ||
for (k, v) in body["data"].items() |
c[0][1].startswith( | ||
"https://test-account.snowflakecomputing.com:443" | ||
) |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 21 days ago
To fix the issue, we should replace the startswith
check with a more robust method that parses the URL and validates its components. Specifically, we can use Python's urllib.parse
module to extract the hostname and scheme from the URL and ensure they match the expected values. This approach eliminates the risk of substring matching errors and ensures that the URL is validated correctly.
The changes will involve:
- Importing the
urlparse
function fromurllib.parse
. - Replacing the
startswith
check with a parsed URL validation that ensures the scheme ishttps
, the hostname matches the expected value, and the port is correct.
-
Copy modified line R841 -
Copy modified lines R844-R846 -
Copy modified line R868 -
Copy modified lines R871-R873
@@ -840,5 +840,8 @@ | ||
) # Skip tear down, there's only a mocked rest api | ||
from urllib.parse import urlparse | ||
assert any( | ||
[ | ||
c[0][1].startswith("https://test-host:443") | ||
urlparse(c[0][1]).scheme == "https" | ||
and urlparse(c[0][1]).hostname == "test-host" | ||
and urlparse(c[0][1]).port == 443 | ||
for c in mocked_fetch.call_args_list | ||
@@ -864,7 +867,8 @@ | ||
) # Skip tear down, there's only a mocked rest api | ||
from urllib.parse import urlparse | ||
assert any( | ||
[ | ||
c[0][1].startswith( | ||
"https://test-account.snowflakecomputing.com:443" | ||
) | ||
urlparse(c[0][1]).scheme == "https" | ||
and urlparse(c[0][1]).hostname == "test-account.snowflakecomputing.com" | ||
and urlparse(c[0][1]).port == 443 | ||
for c in mocked_fetch.call_args_list |
edd918d
to
fd32ae8
Compare
.github/workflows/build_test.yml
Outdated
name: Test asyncio ${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }} | ||
needs: build | ||
runs-on: ${{ matrix.os.image_name }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
os: | ||
- image_name: ubuntu-latest | ||
download_name: linux | ||
python-version: [3.8] | ||
cloud-provider: [aws] | ||
download_name: manylinux_x86_64 | ||
- image_name: macos-latest | ||
download_name: macosx_x86_64 | ||
- image_name: windows-latest | ||
download_name: win_amd64 | ||
python-version: ["3.10", "3.11", "3.12"] | ||
cloud-provider: [aws, azure, gcp] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Set up Python | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: ${{ matrix.python-version }} | ||
- name: Display Python version | ||
run: python -c "import sys; print(sys.version)" | ||
- name: Setup parameters file | ||
shell: bash | ||
env: | ||
PARAMETERS_SECRET: ${{ secrets.PARAMETERS_SECRET }} | ||
run: | | ||
gpg --quiet --batch --yes --decrypt --passphrase="$PARAMETERS_SECRET" \ | ||
.github/workflows/parameters/public/parameters_${{ matrix.cloud-provider }}.py.gpg > test/parameters.py | ||
- name: Download wheel(s) | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: ${{ matrix.os.download_name }}_py${{ matrix.python-version }} | ||
path: dist | ||
- name: Show wheels downloaded | ||
run: ls -lh dist | ||
shell: bash | ||
- name: Upgrade setuptools, pip and wheel | ||
run: python -m pip install -U setuptools pip wheel | ||
- name: Install tox | ||
run: python -m pip install tox>=4 | ||
- name: Run tests | ||
run: python -m tox run -e olddriver | ||
run: python -m tox run -e aio | ||
env: | ||
PYTHON_VERSION: ${{ matrix.python-version }} | ||
cloud_provider: ${{ matrix.cloud-provider }} | ||
PYTEST_ADDOPTS: --color=yes --tb=short | ||
TOX_PARALLEL_NO_SPINNER: 1 | ||
shell: bash | ||
- name: Combine coverages | ||
run: python -m tox run -e coverage --skip-missing-interpreters false | ||
shell: bash | ||
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: coverage_aio_${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }} | ||
path: | | ||
.tox/.coverage | ||
.tox/coverage.xml | ||
|
||
test-noarrowextension: | ||
name: No Arrow Extension Test ${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }} | ||
needs: lint | ||
test-unsupporeted-aio: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 13 days ago
To fix the problem, you should add an explicit permissions
block to the workflow file .github/workflows/build_test.yml
. The best way to do this is to add the block at the top level of the workflow, so it applies to all jobs unless overridden. Since the jobs only need to read repository contents (for checkout and artifact download), the minimal required permission is contents: read
. This change should be made immediately after the name:
and before the on:
block (i.e., after line 1 and before line 3). No additional imports or definitions are needed.
-
Copy modified lines R2-R3
@@ -1,2 +1,4 @@ | ||
name: Build and Test | ||
permissions: | ||
contents: read | ||
|
.github/workflows/build_test.yml
Outdated
# name: Combine coverage | ||
# needs: [lint, test, test-fips, test-lambda, test-aio] | ||
# runs-on: ubuntu-latest | ||
# steps: | ||
# - uses: actions/checkout@v4 | ||
# - uses: actions/download-artifact@v4 | ||
# with: | ||
# path: artifacts | ||
# - name: Set up Python | ||
# uses: actions/setup-python@v4 | ||
# with: | ||
# python-version: '3.9' | ||
# - name: Display Python version | ||
# run: python -c "import sys; print(sys.version)" | ||
# - name: Upgrade setuptools and pip | ||
# run: python -m pip install -U setuptools pip wheel | ||
# - name: Install tox | ||
# run: python -m pip install tox>=4 | ||
# - name: Collect all coverages to one dir | ||
# run: | | ||
# python -c ' | ||
# from pathlib import Path | ||
# import shutil | ||
|
||
combine-coverage: | ||
if: ${{ success() || failure() }} | ||
name: Combine coverage | ||
needs: [lint, test, test-fips, test-lambda] | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/download-artifact@v4 | ||
with: | ||
path: artifacts | ||
- name: Set up Python | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: '3.8' | ||
- name: Display Python version | ||
run: python -c "import sys; print(sys.version)" | ||
- name: Upgrade setuptools and pip | ||
run: python -m pip install -U setuptools pip wheel | ||
- name: Install tox | ||
run: python -m pip install tox>=4 | ||
- name: Collect all coverages to one dir | ||
run: | | ||
python -c ' | ||
from pathlib import Path | ||
import shutil | ||
|
||
src_dir = Path("artifacts") | ||
dst_dir = Path(".") / ".tox" | ||
dst_dir.mkdir() | ||
for src_file in src_dir.glob("*/.coverage"): | ||
dst_file = dst_dir / ".coverage.{}".format(src_file.parent.name[9:]) | ||
print("{} copy to {}".format(src_file, dst_file)) | ||
shutil.copy(str(src_file), str(dst_file))' | ||
- name: Combine coverages | ||
run: python -m tox run -e coverage | ||
- name: Publish html coverage | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
include-hidden-files: true | ||
name: overall_cov_html | ||
path: .tox/htmlcov | ||
- name: Publish xml coverage | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
include-hidden-files: true | ||
name: overall_cov_xml | ||
path: .tox/coverage.xml | ||
- uses: codecov/codecov-action@v4 | ||
with: | ||
files: .tox/coverage.xml | ||
token: ${{ secrets.CODECOV_TOKEN }} | ||
# src_dir = Path("artifacts") | ||
# dst_dir = Path(".") / ".tox" | ||
# dst_dir.mkdir() | ||
# for src_file in src_dir.glob("*/.coverage"): | ||
# dst_file = dst_dir / ".coverage.{}".format(src_file.parent.name[9:]) | ||
# print("{} copy to {}".format(src_file, dst_file)) | ||
# shutil.copy(str(src_file), str(dst_file))' | ||
# - name: Combine coverages | ||
# run: python -m tox run -e coverage | ||
# - name: Publish html coverage | ||
# uses: actions/upload-artifact@v4 | ||
# with: | ||
# include-hidden-files: true | ||
# name: overall_cov_html | ||
# path: .tox/htmlcov | ||
# - name: Publish xml coverage | ||
# uses: actions/upload-artifact@v4 | ||
# with: | ||
# include-hidden-files: true | ||
# name: overall_cov_xml | ||
# path: .tox/coverage.xml | ||
# - uses: codecov/codecov-action@v4 | ||
# with: | ||
# files: .tox/coverage.xml | ||
# token: ${{ secrets.CODECOV_TOKEN }} |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 13 days ago
To fix the problem, we should add a permissions
block to the workflow or to each job that does not already have one. The minimal starting point is contents: read
, which allows the workflow to read repository contents but not write to them. This should be added at the root level of the workflow (top-level, just after the name
and before on
), so it applies to all jobs unless overridden. No additional imports or definitions are needed; this is a YAML configuration change.
-
Copy modified lines R3-R5
@@ -2,2 +2,5 @@ | ||
|
||
permissions: | ||
contents: read | ||
|
||
on: |
c9f892b
to
5f87ff8
Compare
0678964
to
49d5a0a
Compare
49d5a0a
to
b7a65a2
Compare
b7a65a2
to
c77cd31
Compare
4964b53
to
df50e8d
Compare
df50e8d
to
3da273a
Compare
3da273a
to
3d7d6b6
Compare
DO NOT MERGE, this is a workflow debug branch